Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FE] 지출 내역 생성하기 기능 추가 #587

Merged
merged 15 commits into from
Sep 24, 2024
Merged

[FE] 지출 내역 생성하기 기능 추가 #587

merged 15 commits into from
Sep 24, 2024

Conversation

Todari
Copy link
Contributor

@Todari Todari commented Sep 22, 2024

issue

구현 목적

유효성 검증

  • 지출내역 제목과 멤버 이름이 정책과 다르게 입력된 경우 예상치 못한 layout shift가 발생하여 유저 경험이 좋지 못하거나 서버에 옳지 못한 요청을 보냄으로 인해 사용자 측에서 예상치 못한 오류를 맞이할 수 있습니다.
  • 이를 방지하기 위해 유효성 검증을 추가하였습니다.

파일 분리

  • funnel 구조의 각 step을 하나의 파일에서 관리하다 보니 가독성이 좋지 않아 불편함을 느꼈습니다.
  • 또한 각 step이 하는 역할이 조금씩 다르지만, 모두 AddBillFunnel에 구현되어 있어 역할 분리의 필요성을 느꼈습니다.
  • 이를 개선하기 위해 파일을 분리하고 각 step이 하는 역할에 대해서 분리해 주고자 합니다.

구현 내용

AddBillFunnel.tsx

const AddBillFunnel = () => {
  const {currentMembers} = useRequestGetCurrentMembers();
  const [step, setStep] = useState<BillStep>('price');
  const [billInfo, setBillInfo] = useState<BillInfo>({
    price: '',
    title: '',
    members: [],
  });

  useEffect(() => {
    currentMembers && setBillInfo(prev => ({...prev, members: currentMembers}));
  }, [currentMembers]);

  return (
    <MainLayout backgroundColor="white">
      <TopNav>
        <Back />
      </TopNav>
      {step === 'price' && <PriceStep billInfo={billInfo} setBillInfo={setBillInfo} setStep={setStep} />}
      {step === 'title' && <TitleStep billInfo={billInfo} setBillInfo={setBillInfo} setStep={setStep} />}
      {step === 'members' && (
        <MembersStep billInfo={billInfo} setBillInfo={setBillInfo} setStep={setStep} currentMembers={currentMembers} />
      )}
    </MainLayout>
  );
};
  • 각 step을 component로 분리하고, 공통적으로 필요한 정보들을 props로 넘겨주었습니다.

TitleStep.tsx

// ...
  const onTitleInputChange = (value: string) => {
    if (REGEXP.billTitle.test(value)) {
      setBillInfo(prev => ({...prev, title: value}));
    }
  };

  const handleTitleInputChange = (event: React.ChangeEvent<HTMLInputElement>) => {
    if (event.target.value.length > 12) {
      setErrorMessage('지출내역은 12자까지 입력 가능해요');
      onTitleInputChange(billInfo.title.slice(0, 12));
    } else {
      setErrorMessage('');
      onTitleInputChange(event.target.value);
    }
  };

  const canSubmitTitleInput = billInfo.title && !errorMessage;

  const handleTitleInputEnter = (event: React.KeyboardEvent<HTMLInputElement>) => {
    if (event.nativeEvent.isComposing) {
      return;
    }
    if (event.key === 'Enter' && canSubmitTitleInput) {
      event.preventDefault();
      setStep('members');
    }
  };
// ...
  • /^([ㄱ-ㅎ가-힣a-zA-Z0-9ㆍᆢ]\s?)*$/, 정규식에 맞지 않으면 title을 입력할 수 없습니다.
  • title이 12글자가 넘으면 error를 발생시키고, 다음 단계로 이동할 수 없습니다.
  • 다음단계로 이동할 수 없는 것은 FixedButton을 클릭하는 것과 Enter 키를 입력하는 것 모두 해당합니다.

MemberStep.tsx

//...
  const onNameInputChange = (value: string) => {
    if (REGEXP.memberName.test(value)) {
      setNameInput(value);
    }
  };

  const handleNameInputChange = (event: React.ChangeEvent<HTMLInputElement>) => {
    if (event.target.value.length > 4) {
      setErrorMessage('이름은 4자까지 입력 가능해요');
      onNameInputChange(nameInput.slice(0, 4));
    } else {
      setErrorMessage('');
      onNameInputChange(event.target.value);
    }
  };
//...
  • /^([ㄱ-ㅎ가-힣a-zA-Zㆍᆢ]\s?)*$/, 정규식에 맞지 않으면 name을 입력할 수 없습니다.
  • name이 4글자가 넘으면 error를 발생시키고, 다음 단계로 이동할 수 없습니다.
  • 다음 단계로 이동할 수 없는 것은 Enter키를 입력해서 member를 추가하는 행동을 의미합니다.
//...
  const setBillInfoMemberWithId = (name: string) => {
    const existingMember = currentMembers.find(currentMember => currentMember.name === name);
    if (existingMember) {
      setBillInfo(prev => ({...prev, members: [...prev.members, {id: existingMember.id, name: name}]}));
    } else {
      setBillInfo(prev => ({...prev, members: [...prev.members, {id: -1, name: name}]}));
    }
  };
//...
  • member가 이미 존재할 때 (currentMember에 있을 때) currentMember의 Id를 사용하고, 그렇지 않으면 -1 을 Id에 추가합니다.
//...
  const handlePostBill = async () => {
    if (billInfo.members.map(({id}) => id).includes(-1)) {
      postMembers({
        members: billInfo.members.filter(member => member.id === -1),
      });
    } else {
      postBill({
        title: billInfo.title,
        price: Number(billInfo.price.replace(',', '')),
        members: billInfo.members.map(({id}) => id),
      });
    }
  };

  useEffect(() => {
    if (isSuccessPostMembers && responseMemberIds) {
      postBill({
        title: billInfo.title,
        price: Number(billInfo.price.replace(',', '')),
        members: billInfo.members.map(member =>
          member.id === -1 ? responseMemberIds.members.find(m => m.name === member.name)?.id || member.id : member.id,
        ),
      });
    }
  }, [isSuccessPostMembers, responseMemberIds]);

  useEffect(() => {
    if (isSuccessPostBill) {
      navigate(`/event/${eventId}/admin`);
    }
  }, [isSuccessPostBill]);
//...
  • 작성 완료 버튼을 누르면 memberId-1이 있는지 확인합니다.
  • -1 id가 있다면, 현재 선택된 member들 중 id가 없는 이름들을 배열로 postMembers API를 요청합니다.
  • 요청에 성공했다면, posetMembers API 응답으로 얻은 memberId를 통해 postBill API 를 요청합니다.
  • id가 -1이 없었다면, 현재 선택된 member들을 이용해 바로 postBill API 를 요청합니다.

TroubleShooting

  • TitleStep에서 이전으로 버튼을 눌러 PriceStep으로 이동했을 때, NumberKeyboard가 초기화 되던 문제

  • NumberKeyboard가 렌더링되면서, useNumberKeyboard 내부의 value가 ''으로 초기화되는 것이 원인

  • NumberKeyboard에 initialValue를 추가함으로써 이 문제를 해결

  • before

// usenumberKeyboard.tsx
const useNumberKeyboard = ({type, maxNumber, onChange}: Props) => {
  const [value, setValue] = useState('');
// ...
  • after
// usenumberKeyboard.tsx
const useNumberKeyboard = ({type, maxNumber, initialValue, onChange}: Props) => {
  const [value, setValue] = useState(initialValue ?? '');
// ...

논의하고 싶은 부분(선택)

  • postBill 부분의 API 설계상 새로운 멤버가 생겼다면 요청을 2번 보내야 하다 보니 로직적으로 복잡해 진 부분이 있는 것 같아요. 이부분에 대해서 어떻게 생각하시는지, 좋은 방법이 있으신지 궁금합니다~!

@Todari Todari added 🖥️ FE Frontend ⚙️ feat feature labels Sep 22, 2024
@Todari Todari self-assigned this Sep 22, 2024
Copy link

Copy link

Copy link

Copy link

Copy link

Copy link

Copy link

Copy link

@jinhokim98 jinhokim98 added this to the lev4 milestone Sep 23, 2024
Copy link

Copy link
Contributor

@jinhokim98 jinhokim98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생했어요~! 코멘트를 다는 도중 잘 개선해줘서 바로 approve 달게요! 빠이팅

const {theme} = useTheme();
const amountKeypads = ['1', '2', '3', '4', '5', '6', '7', '8', '9', '00', '0', '<-'];
const numberKeypads = ['1', '2', '3', '4', '5', '6', '7', '8', '9', '', '0', '<-'];

const {onClickKeypad, onClickDelete, onClickDeleteAll, onClickAddAmount} = useNumberKeyboard({
type,
initialValue,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

생각보다 금방 해결이 돼서 다행이에요~

@@ -7,5 +7,5 @@ export const ROUTER_URLS = {
eventLogin: '/event/:eventId/login',
eventManage: '/event/:eventId/admin',
home: '/event/:eventId/home',
addBill: 'event/:eventId/addBill',
addBill: '/event/:eventId/add-bill',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍👍

Comment on lines +18 to +24
useEffect(() => {
document.body.style.overflow = 'hidden';

return () => {
document.body.style.overflow = 'auto';
};
}, []);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오~~~ 정말 디테일한 구현 좋아요!
이 내용을 퍼널을 이용하는 다른 곳에서도 자동 적용이 되도록 처리를 해줘도 좋을 것 같아요.

Comment on lines +53 to +55
const canAddMembers = nameInput && !errorMessage;

const canSubmitMembers = billInfo.members.length !== 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

boolean 조건 상수화👍👍

Comment on lines 86 to 93
members: billInfo.members.map(member =>
member.id === -1 ? newMembers.members.find(m => m.name === member.name)?.id || member.id : member.id,
),
});
} else {
postBill({
title: billInfo.title,
price: Number(billInfo.price.replace(',', '')),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

새로운 멤버가 있는 경우 새로운 멤버를 postMembers를 통해 값을 받아온 뒤 postBill을 하고, 새로운 멤버가 없을 경우 postBill를 바로 실행하는 구조인 것 같아요.

지금의 구조는 postMembers를 보낸 후 useEffect를 이용해서 isSuccess상태가 될 때 요청을 날리는 방식인데, mutateAsync를 사용해서 postMembers를 await으로 기다린 후 postBill을 요청하면 더 보기 좋게 작성할 수 있을 것 같아요

Comment on lines +12 to +25
const usePriceStep = ({setStep, setBillInfo}: Props) => {
const handleNumberKeyboardChange = useCallback(
(value: string) => {
setBillInfo(prev => ({...prev, price: value}));
},
[setBillInfo],
);

const handleNextStep = () => {
setStep('title');
};

return {handleNumberKeyboardChange, handleNextStep};
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

step별로 훅을 분리하니 더 직관적으로 보이는 것 같아요

Copy link

Copy link

Copy link

@Todari Todari merged commit 8271194 into fe-dev Sep 24, 2024
2 checks passed
@Todari Todari deleted the feature/#585 branch September 24, 2024 07:42
@Todari Todari added this to the v2.0.0 milestone Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

3 participants